chore: Clean up testing framework#2542
Open
trisyoungs wants to merge 24 commits into
Open
Conversation
trisyoungs
force-pushed
the
dissolve2/remove-coredata
branch
from
4fc7e40 to
2aca81c
Compare
trisyoungs
changed the title
trisyoungs
force-pushed
the
dissolve2/remove-coredata
branch
from
aa4b541 to
6d88fb4
Compare
trisyoungs
force-pushed
the
dissolve2/clean-up-testing-framework
branch
from
254f7d5 to
903ba78
Compare
rprospero
reviewed
Jun 30, 2026
Contributor
There was a problem hiding this comment.
I've left a couple of comments about predicate assertsions that might give us cleaner results from our tests.
Comment on lines
+79
to
+84
| void testVec3(const Vector3 &A, const Vector3 &B, double tolerance) | ||
| { | ||
| EXPECT_NEAR(A.x, B.x, tolerance); | ||
| EXPECT_NEAR(A.y, B.y, tolerance); | ||
| EXPECT_NEAR(A.z, B.z, tolerance); | ||
| } |
Contributor
There was a problem hiding this comment.
One pain point that I inflicted on myself while doing the parsers work is putting EXPECT statements in a wrapper function. The problem is that, when the test fails, it will just tell you that a line in the wrapper function failed without telling you which line in your actual test called that wrapper function. Returning a predicate assertion, like I explained above, ensure that the test runner reports back the actual place that the line failed.
Co-authored-by: Adam Washington <adam.washington@stfc.ac.uk>
trisyoungs
force-pushed
the
dissolve2/clean-up-testing-framework
branch
from
9cef717 to
c8ba80d
Compare
added 2 commits
July 3, 2026 14:09
rprospero
approved these changes
Jul 6, 2026
Comment on lines
+424
to
+448
| // Compare TOML values with context, but without insisting on a specific ordering of fields | ||
| void compareToml(std::string location, SerialisedValue toml, SerialisedValue toml2) | ||
| { | ||
| if (toml.is_table()) | ||
| { | ||
| ASSERT_TRUE(toml2.is_table()) << location; | ||
| for (auto &[k, v] : toml.as_table()) | ||
| { | ||
| ASSERT_TRUE(toml2.contains(k)) << location << "." << k << std::endl << "Expected:" << std::endl << toml[k]; | ||
| compareToml(std::format("{}.{}", location, k), v, toml2.at(k)); | ||
| } | ||
| } | ||
| else if (toml.is_array()) | ||
| { | ||
| auto arr = toml.as_array(); | ||
| auto arr2 = toml2.as_array(); | ||
| ASSERT_EQ(arr.size(), arr2.size()) << location << std::endl << "Expected" << std::endl << toml; | ||
| for (int i = 0; i < arr.size(); ++i) | ||
| compareToml(std::format("{}[{}]", location, i), arr[i], arr2[i]); | ||
| } | ||
| else | ||
| { | ||
| EXPECT_EQ(toml, toml2) << location; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
It's outside the scope of this PR, but this should definitely be reworked to return an assertion predicate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a big clean-up PR of the test classes we had, hopefully making things a little clearer and setting up for follow-on work with
Nodetest fixtures.